Fix registration error when setup by archlitchi · Pull Request #92 · Project-HAMi/ascend-device-plugin

archlitchi · 2026-06-30T10:16:38Z

Add prefix 'passthrough:///' to avoid connection error

Summary by CodeRabbit

Bug Fixes
- Improved local plugin connections over Unix sockets by creating the underlying gRPC client with a passthrough target derived from the socket path.
- Plugin startup now reports connection-creation failures instead of silently ignoring them.
- Connection readiness is handled more robustly: the system waits until the connection is ready, fails fast on transient/shutdown states, and returns a clearer timeout error (instead of a generic context timeout).

Signed-off-by: root <root@aio-node74-arm.cluster.local>

hami-robot · 2026-06-30T10:16:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: archlitchi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [archlitchi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coderabbitai · 2026-06-30T10:16:58Z

📝 Walkthrough

Walkthrough

PluginServer.dial now creates the gRPC client with a passthrough:/// Unix socket target, returns client निर्माण errors, polls connection state until connectivity.Ready, and emits a specific timeout error on deadline.

Changes

gRPC Dial Fix

Layer / File(s)	Summary
gRPC dial target, error handling, and state wait `internal/server/register.go`	Builds a `passthrough:///` target from the Unix socket path, checks `grpc.NewClient` error, polls connection state until `connectivity.Ready`, fails fast on `TransientFailure` or `Shutdown`, and returns a formatted timeout error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐇 I hopped through the dial with a passthrough trail,
Ready came clear, no errors to veil.
On timeout I whisper a path-specific song,
The socket connection now knows where it belongs.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is clearly related to the change: it describes a setup/registration error fix, which matches the registration connection update.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix_register

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/server/register.go`:
- Around line 177-180: The registration flow in NewClient is waiting on
connectivity.Idle before any dialing starts, so it can hang until the context
expires. In the connection setup path that uses grpc.NewClient, call c.Connect()
first, then wait for the channel to reach connectivity.Ready before returning.
Keep the existing timeout/error handling and close the client on failure.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 38ba0b82-dad3-4aff-9073-d853ca0bb838

📥 Commits

Reviewing files that changed from the base of the PR and between ce9ab91 and b54df69.

📒 Files selected for processing (1)

internal/server/register.go

Signed-off-by: root <root@aio-node74-arm.cluster.local>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/server/register.go`:
- Around line 183-186: The connection wait logic in registerServer currently
treats connectivity.TransientFailure as terminal, but it should keep waiting
until the deadline and only exit early on connectivity.Shutdown. Update the
state-handling branch in the connect/wait loop so TransientFailure is ignored or
retried, while Shutdown still closes the connection and returns an error; use
the existing c.Close, state checks, and deadline-based wait flow to locate the
fix.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 143e6ae1-cc46-41bc-966b-e01f1d1cda70

📥 Commits

Reviewing files that changed from the base of the PR and between b54df69 and 0920f23.

📒 Files selected for processing (1)

internal/server/register.go

update_register

b54df69

Signed-off-by: root <root@aio-node74-arm.cluster.local>

hami-robot Bot added the dco-signoff: yes label Jun 30, 2026

hami-robot Bot requested a review from DSFans2014 June 30, 2026 10:16

hami-robot Bot added approved size/S labels Jun 30, 2026

coderabbitai Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread internal/server/register.go Outdated

update

0920f23

Signed-off-by: root <root@aio-node74-arm.cluster.local>

coderabbitai Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread internal/server/register.go

archlitchi merged commit 9f91d30 into main Jun 30, 2026
10 of 11 checks passed

archlitchi deleted the fix_register branch June 30, 2026 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix registration error when setup#92

Fix registration error when setup#92
archlitchi merged 2 commits into
mainfrom
fix_register

archlitchi commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

hami-robot Bot commented Jun 30, 2026

Uh oh!

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

archlitchi commented Jun 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

hami-robot Bot commented Jun 30, 2026

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

archlitchi commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading